chore (client): pin Zod to 3.21.1 to avoid code generator bug in client #700
Conversation
There was a problem hiding this comment.
Hi @fooware, thanks for your contribution!
The e2e tests and linearlite should indeed also be pinned to version 3.21.1 of Zod.
The fact that they are using a higher version of Zod is an accident.
Could you also generate a changeset for this PR please?
To do that, run pnpm changeset at the root of the monorepo.
kevin-dp
changed the title
Done.
Done, but please make sure I did it correctly. Please note that I didn't rebuild any packages here. I assume that a build server handles that during release? |
.changeset/happy-lobsters-type.md
Outdated
| @@ -0,0 +1,8 @@ | |||
| --- | |||
| "electric-sql": patch | |||
| "@core/electric": patch | |||
There was a problem hiding this comment.
The changeset is not correct. It patches all packages but your PR only touches the typescript client (i.e. electric-sql) and the generator (i.e. @electric-sql/prisma-generator). It does not touch Electric (@core/electric) neither does it touch the starter ("create-electric-app": patch).
So the changeset should look like this:
---
electric-sql": patch
@electric-sql/prisma-generator": patch
---
Pin Zod to version 3.21.1
You probably accidentally selected all packages instead of only the changed packages.
There was a problem hiding this comment.
I assumed it only listed the ones it detected changes in so I accepted the defaults.
I’ll push an updated one tomorrow.
There was a problem hiding this comment.
Yep, since you changed the ts-client and the generator it should ask something like:
Which packages would you like to include? …
◯ changed packages
◯ electric-sql
◯ @electric-sql/prisma-generator
◯ unchanged packages
◯ @core/electric
◯ create-electric-appAnd you should only check "changed packages" (by pressing the spacebar).
There was a problem hiding this comment.
@kevin-dp : It didn't show it like that for me, but perhaps that is me not branching in some expected way.
I have now re-generated the changeset as per your instructions.
There was a problem hiding this comment.
@kevin-dp, any feedback? I assume I shouldn’t resolve this thread?
There was a problem hiding this comment.
Hi @fooware, sorry for the delay. We are discussing this internally to see whether we want to pin Zod to a specific version (which affects all user projects) or if we can provide a less invasive fix. Will keep you posted today hopefully!
There was a problem hiding this comment.
@kevin-dp, any feedback? I assume I shouldn’t resolve this thread?
We will need a bit more time to see if we can provide a fix for this under the hood such that we don't need to pin the version of Zod to a specific version for all Electric projects. This will require some changes in the generator. I'm hoping to look into it tomorrow and ideally push a fix for it tomorrow. We will keep you posted here.
There was a problem hiding this comment.
The zod issue is open for some time now and has still not been confirmed, so I think there will not be a fix any time soon. I circumvented the issue by adding an option to use type assertions in zod-prisma-types. It is a brute force method so I can personally use it with latest zod versions and it bypasses all the nice type inference, but since the library is about runtime checks - and these seem to work - it is an acceptable risk I'm willing to take imost of the time
I don't know if this is a valid option in your case but since the types worked pre 3.21.2 there seems to be nothing wrong with the prisma types and with asserting them.
This PR fixes the problem where the generated client may contain type errors for models containing relations. This is a known problem with Zod and Prisma and is described here: chrishoermann/zod-prisma-types#98. That issue also proposes a solution with a dirty type cast: chrishoermann/zod-prisma-types#98 (comment) which is implemented in this PR. This PR removes the need for pinning the version of Zod (#700).
|
@kevin-dp @samwillis does #727 address this issue? |
…1187) Every Electric app needs Zod in order to use the generated client. However, in recent versions of Zod something changed that breaks the types in the generated client (cf. #700). Therefore, Electric apps should always use Zod version 3.21.1. Added this as a peer dependency such that npm complains if the app installs a different version. Similarly, added an optional peer dependency for Prisma v4 such that it complains if the app installs another version of Prisma.
Summary
As discussed on Discord with @kevin-dp, it appears that the dependencies for Zod can pull in a version that causes the generated client to contain invalid casts.
The issue in Zod + Prisma can be found here:
colinhacks/zod#2184
chrishoermann/zod-prisma-types#98
Background
For us, the problem manifests itself by adding any kind of basic table relationship and generating a client. Indeed, it appears to also happen in this example code created by a third party:
https://github.com/KyleAMathews/vite-react-router-electric-sql-starter/tree/main
In both that example, and our internal code, it is enough to comment out any relationships and the generated code is fine, or rather doesn't contain the code blocks that are causing the issues.
Patching the packages lock file in both cases to resolve Zod to 3.21.1 (as suggested in the linked issue threads) fixed the issue for both our code and the third-party example.
Temporary Workaround
Since we are using PNPM we could achieve the same result by adding a forced version override to our root package.json file:
Using that we could also experiment with different Zod versions, and indeed, going to 3.21.2 or higher reintroduced the problem.
Suggested Fix
This PR pins the version of Zod to 3.21.1 which is the latest version that did not cause the issue. Hopefully, this will mean that new users will get a local package lock file that resolves Zod to 3.21.1 and a working client generator. That last part is a bit hard for me to verify locally though.
Caveat
I have only updated the TypeScript Client and the Client Generator, but there are at least two places in this mono repo that explicitly reference higher versions of Zod:
^3.21.4^3.22.2I was not comfortable downgrading those dependencies since I do not know why they require a higher version.